Skip to content

fix: error when edited cell is out of grid viewport#2148

Merged
AkshatJawne merged 3 commits intodeephaven:mainfrom
AkshatJawne:2087_edited_cell_viewport_fix
Jul 17, 2024
Merged

fix: error when edited cell is out of grid viewport#2148
AkshatJawne merged 3 commits intodeephaven:mainfrom
AkshatJawne:2087_edited_cell_viewport_fix

Conversation

@AkshatJawne
Copy link
Copy Markdown
Contributor

@AkshatJawne AkshatJawne commented Jul 16, 2024 •
edited
Loading

Resolves #2087

Steps Moving Forward:

  • Clarify whether we want to be able to handle edits that are not in the current viewport -- with my understanding, this will mean we will have to load in more grid data, which does not seem overly valuable (in terms of the efficiency tradeoff) since we are dealing with an improbable edge case where the user begins to edit in the viewport, continues editing outside of the viewport where the cell originally was, and then wants to commit those changes outside as well

** A better solution for the edge case above IMO is to create a new enhancement ticket where we attempt to have the editing cell be floating/sticky in the grid viewport, similar to how it is done on Google Sheets **

Ticket Created: #2153

Screenshot 2024-07-16 at 10 37 02 AM

@AkshatJawne AkshatJawne requested a review from mofojed July 16, 2024 16:39
@AkshatJawne AkshatJawne self-assigned this Jul 16, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 16, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 46.62%. Comparing base (0da547d) to head (9fd7a75).
Report is 2 commits behind head on main.

Files Patch % Lines
packages/grid/src/Grid.tsx 71.42% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2148      +/-   ##
==========================================
- Coverage   46.62%   46.62%   -0.01%     
==========================================
  Files         685      685              
  Lines       38492    38497       +5     
  Branches     9800     9592     -208     
==========================================
+ Hits        17948    17950       +2     
- Misses      20492    20537      +45     
+ Partials       52       10      -42
Flag Coverage Δ
unit 46.62% <71.42%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mofojed
mofojed requested changes Jul 17, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small cleanup.

Feel free to file an feature request for the floating google docs type behaviour you've described. With the floating cell we'd want to display the cell coordinates next to the floating input, but that could get cluttered with a long column name, and also we don't display the row headers by default. Just to note on the ticket.

image

Comment thread packages/grid/src/Grid.tsx Outdated
@AkshatJawne AkshatJawne requested a review from mofojed July 17, 2024 16:32
@AkshatJawne AkshatJawne merged commit 3fccd43 into deephaven:main Jul 17, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mofojed mofojed mofojed approved these changes

Assignees

@AkshatJawne AkshatJawne

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error when Edited Cell scrolls out of Viewport

2 participants

@AkshatJawne @mofojed
⚡